Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev master candidate 2018 06 03 #787

Merged
merged 54 commits into from
Jun 10, 2018
Merged

Conversation

adcroft
Copy link
Collaborator

@adcroft adcroft commented Jun 5, 2018

This PR to dev/master includes fixes for the various reverts made last week when we got ahead of ourselves inadvertently.

There are not too many differences here:

  • The lion's share of changes are in src/ice_shelf/ which is not widely used.
  • The next biggest difference is moving legacy_diabatic_driver() back in to MOM_diabatic_driver.F90 after we gave inaccurate suggestions.
  • The majority of the rest are documentation or cleanup.

@kshedstrom Can you check you fix is still in here? (I can see it but that's not the same as it working for you...)
@gustavo-marques Likewise, double check I didn't lose anything of yours.

We'll wait for thumbs up from the usual suspects...

MJHarrison-GFDL and others added 30 commits May 15, 2018 11:14
  Corrected the code setting p_surf in MOM_ice_shelf so that the ISOMIP test
case gives the same answers before and after a separate mec_forcing type
structure was added.  This restores the answers in two ISOMIP test cases to the
previous commit.
  Added corrections for the ISOMIP and related test cases at the point in the
code where additional problems for this case were introduced.
  Added a new element, accumulate_rigidity, to the mech_forcing type to control
whether rigidity is reset or accumulated in various ice elements.  With this
change, the ISOMIP test cases return to acceptable solutions; other cases are
unchanged.
  Improved the post_data peculiar size error messsages, so that they now give
information about which diagnostic is being posted, the understood sizes, and
the strange size that has been sent in.  All answers are bitwise identical.
  Corrected index ranges for reproducing_sum in MOM_tracer_chkinv.  Because the
tracer array is passed into reproducing sum as an array, it is converted
internally to start at 1, per F90 conventions.  This is now compensated for in
the tracer range arguments.  The solutions are identical, as are the tracer
inventories if the data domains start at 1, as is common with MOM6.
  Added a new element, accumulate_p_surf, to the mech_forcing type, to indicate
whether surface pressure has been reset to 0 and can be accumulated across
multiple contributions, or whether it should be reset if it is to be changed.
All answers in existing test cases are bitwise identical.
  Separated the new publicly visibile subroutine add_shelf_forces out of
add_shelf_flux, permitting the dynamic forces to be set separately from the
thermodynamic forcing.  All answers are bitwise identical.
  Eliminated the unused triangular finite element subroutines and related
arrays.  Also added grid-type arguments to numerous internal subroutines, and
used this to set array sizes. Eliminated the variable isym and the macros
N[IJ]LIMB_SYM_ and [IJ]SUMSTART_INT_. Also eliminated the finite-element shape
argument, FE, from some subroutines.  All test cases are bitwise identical.
  Moved the ice shelf state variables, mass_shelf, area_shelf_h, h_shelf and
hmask into a new ice_shelf_state type in the new module MOM_ice_shelf_state, and
use this type for these variables in MOM_ice_shelf.F90.  The allocation and
deallocation of this new type is handled via calls to ice_shelf_state_init and
ice_shelf_state_end, respectively.  This change will permit the ice shelf
dynamics code to be separated out from the rest of the ice shelf code. All
answers are bitwise identical.
  Use elements of the ice_shelf_state for the thermodynamic fluxes between the
ice shelf and the ocean, as seen by the ice shelf. Also made the exchange
velocity arrays into local variables.  All answers are bitwise identical.
  Replaced the excessive use of pointers and allocatable arrays in
MOM_ice_shelf.F90 with automatically allocated arrays using information from the
grid type to set the array extents.  Because pointers are not being used, many
of the arguments to the internal subroutines have been changed from pointers to
simple arguments with an intent, while other arguments have been added to
explicitly pass the arrays being worked on in preparation for splitting out the
ice shelf dynamics.  The remaining pointers are nullified where they are
declared.  All answers are bitwise identical.
  Created a new ice shelf dynamics control structure, separate from the overall
ice shelf control structure, in preparation for moving the ice shelf dynamics
into its own module.  All answers are bitwise identical, although several
internal interfaces are changed.
  Made a number of minor changes in MOM_ice_shelf.F90, including renaming a
number of excessively long variable names, eliminating unused variables, and
adding dOxyGen comments to some arguments and routines, and fixing code
indentation in some routines.   The suffix _bilinear is no longer needed in
subroutine names and has been removed.  All answers are bitwise identical.
Insufficient testing of N-S OBCs for all options.
Hallberg-NOAA and others added 24 commits May 25, 2018 09:27
  Created a new subroutine, initialize_ice_shelf_dyn, to initialize the ice
shelf dynamics control structure.  Also commented out the register_diag_field
call for ice_surf, which was never actually posted.  Moved the IDs for
diagnostics related to the ice shelf dynamics into the ice shelf dynamics
control structure.  Also dOxyGenised the arguments to initialize_ice_shelf. All
answers are bitwise identical in the MOM6_examples test cases, but the order of
entries in the MOM_parameter_doc files will change with active ice shelf
dynamics.
This reverts commit bcb3f12, reversing
changes made to 496ab52.

- Unfortunately, PR #776 included a merge from dev/ncar as a result of PR #777
  and both these PRs were made on branches that were updated subsequent to the
  instigation of tests. The tests passed but the branches had updates that
  were not tested. In both cases, the untested updates would not have passed so I
  am revoked the entirety of #776 in order to recover a working code.
- The creation of legacy_diabatic() was made by putting it into a
  separate module which require making members of diabatic_CS public.
- I've moved legacy_diabatic() into MOM_diabatic_driver.F90 and made
  diabatic_CS private. This also removes some duplicated code for
  diagnostics.
- The call to diabatic_driver_end() was commented out.
- deallocation should only happen if allocation was done
  Created the new routine update_ice_shelf and moved 7 elements of the ice
shelf control structure into the ice shelf dynamics control structure.  Also
moved several of the post data calls for ice shelf diagnostics into this new
routine, so they will only occur with active ice shelf dynamics.  All solutions
are bitwise identical, but there will be changes in the MOM_parameter_doc
and available diagnostics files.
  Created a new module for the ice shelf dynamics, separating numerous routines
out from MOM_ice_shelf.  All answers are bitwise identical, although there are
several new publicly visible subroutines.
  Fixed a number of instances in MOM_ice_shelf.F90 that did not use the MOM6
standard 2-point indentation.  Also removed some trailing white space. All
answers are bitwise identical.
  Separated out the call to add_shelf_forces from add_shelf_fluxes and
eliminated the mech_forcing type argument to add_shelf_fluxes.  All answers
are bitwise identical.
  Made forces into an optional argument for shelf_calc_flux, and then added
calls to add_shelf_forces in most places where shelf_calc_flux is called.  Also
added acculumulate_p_surf as an element in the forcing type, as well as the
mech_forcing_type, so that surface pressure can be calculated independently in
the two types, and added a new internal routine, add_shelf_pressure, in the
MOM_ice_shelf module.  All answers are bitwise identical in the test cases.
- The FMS code that compares checksums in files has a dummy argument of dimension(3)
  but MOM6 was passing a dimension(1) variable. Only the first entry seems to be
  non-zero which is why things seemed to work BUT in debug mode we were hitting an
  out-of-array-bounds condition.
Insufficient testing of N-S OBCs for all options.
Changed dimensions of checksum_file to 3
  dOxyGenized numerous arguments and cleaned up code and variable names in
various auxiliary ice_shelf code.  All answers are bitwise identical.
  Added get_ALE_sponge_nz_data and get_ALE_sponge_thicknesses, to provide an
interface to get information about the fixed ALE sponge grid.  All answers
are bitwise identical.
  Added dOxyGenized comments the arguments in MOM_ice_shelf_dynamics.F90.
Because I do not fully understand the ice-sheet dynamics model, these should be
reviewed and revised by someone who understands the ice sheet dynamics solver.
All answers in the test cases are bitwise identical.
- This reverts commit 2c9bf18 in order to
  merge in dev/master which included changes that were reverted.
- The revert was temporary until subsequent commits were made to address
  issues.
- Gaea runtime variability is causing numerous timeouts again
  so rather than assuming the submitted job succeeds we now test that
  the last file to be made exists.
- Also added 2 minutes to job.
@kshedstrom
Copy link
Collaborator

kshedstrom commented Jun 5, 2018 via email

@gustavo-marques
Copy link
Collaborator

I approve this PR.

@adcroft adcroft merged commit d1ceed0 into dev/master Jun 10, 2018
@adcroft adcroft deleted the dev-master-candidate-2018-06-03 branch July 15, 2018 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants